SDK-108 Route registerToken through OfflineRequestProcessor for network-failure retry#1068
Conversation
…rk-failure retry Brings registerToken to feature parity with disablePush (shipped under SDK-297): the registration request is now persisted to the offline task queue and replayed when the network returns, instead of being dropped fire-once on transient network failures. Implementation mirrors the SDK-297 disableDevice retrofit: - Adds `register(...)` to `RequestProcessorProtocol` (taking the already-resolved `notificationsEnabled: Bool`, not the async notification-state provider) so both processors can implement it and `sendUsingRequestProcessor` can pick offline vs online at call time. - Adds `RequestIdentifier.registerToken` (sibling to the existing `disableDevice` identifier). - Implements `register(...)` in `OfflineRequestProcessor` using the existing `RequestCreator.createRegisterTokenRequest(...)` + `sendIterableRequest(...)` pattern. - Updates `OnlineRequestProcessor.register(...)` to conform to the new protocol signature and consume the shared identifier; the existing private register helper is folded into the public one. The notification-state wrinkle (parallel to SDK-297's `UserIdentitySnapshot`): `notificationStateProvider.isNotificationsEnabled` is now resolved inside `RequestHandler.register` BEFORE routing to a processor. The resolved bool is captured into the offline task body at schedule time, so a request queued offline and replayed later carries the bool as it was at call time — replay does NOT re-query notification state. Public IterableAPI / InternalIterableAPI surface and Obj-C selectors are unchanged. Tests in `offline-events-tests` cover (a) offline replay with the expected body, and (b) the snapshot semantic — a `MockNotificationStateProvider` flipped from `true` to `false` after register is called still yields `notificationsEnabled: true` on the replayed request. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| onSuccess: onSuccess, | ||
| onFailure: onFailure) | ||
| notificationStateProvider.isNotificationsEnabled { notificationsEnabled in | ||
| _ = self.sendUsingRequestProcessor { processor in |
There was a problem hiding this comment.
On master, register went straight to OnlineRequestProcessor and built the body synchronously after isNotificationsEnabled, so setCurrentUser read auth essentially at call time. After this PR, the build runs inside sendUsingRequestProcessor's deferred closure (after canSchedule() + a queue hop), and createRegisterTokenRequest still reads live authProvider.auth rather than a captured UserIdentitySnapshot like disableDeviceForCurrentUser does. Intentional, or should register mirror the SDK-297 snapshot pattern?
There was a problem hiding this comment.
Mirrored the snapshot pattern in fb361ff. register now captures UserIdentitySnapshot at call time (before the async notification hop) and threads it into createRegisterTokenRequest, same as disableDeviceForCurrentUser. Tests cover offline-replay and online-fallback keeping the call-time identity. 36/0.
There was a problem hiding this comment.
Optional UserIdentitySnapshot? on six signatures + a forked branch in RequestCreator feels heavy for a race that only exists in RequestHandler.register's async hop. The disableDevice symmetry is also partial, those tasks survive handleLogout(), register doesn't. Can we fix it where the gap actually is instead of threading a parameter through every layer below it?
There was a problem hiding this comment.
Simplified in eed546e. Dropped the cross-layer snapshot threading: capture call-time auth once in register, carry it on RegisterTokenInfo.auth, and RequestCreator resolves it through the existing single body path. ApiClient, both processors, and the protocol no longer carry the param. You were right on handleLogout, register tasks are purged on logout (only disableDevice is preserved), so the full symmetry was not needed. Kept a small identity guard in register because dropping it regresses onFailure on the offline no-user path. 36/0.
Address review: register now captures a UserIdentitySnapshot synchronously before the async notification-state callback and deferred processor selection, mirroring disableDeviceForCurrentUser. This ensures the register request targets the user who was current at call time rather than whoever live auth points to when the deferred/offline build resolves. The snapshot threads through both the online and offline paths into RequestCreator.createRegisterTokenRequest, which applies it (and derives preferUserId from the .userId case) instead of reading live auth. Callers that pass no snapshot keep the previous live-auth behavior and the auth-missing guard. Tests: register at call time then mutate/clear identity before the deferred callback resolves; assert both offline-replay and online-fallback requests still carry the call-time identity and preferUserId. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1068 +/- ##
==========================================
+ Coverage 71.31% 71.42% +0.10%
==========================================
Files 112 113 +1
Lines 9389 9410 +21
==========================================
+ Hits 6696 6721 +25
+ Misses 2693 2689 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Drop the cross-layer UserIdentitySnapshot threading. Capture call-time auth once in RequestHandler.register and carry it on RegisterTokenInfo.auth; RequestCreator resolves registerTokenInfo.auth ?? self.auth through the existing single body path. Reverts the snapshot parameter on ApiClient, the processors, and the protocol. Register offline tasks are purged on logout (only disableDevice is preserved), so the full disableDevice snapshot symmetry was unnecessary. Keep an upfront identity guard in register: dropping it would regress onFailure on the offline no-user path, since the callback only attaches after a task is scheduled. Build green; offline RequestHandlerTests 36/0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
What
Brings
registerTokento feature parity withdisablePush(shipped under SDK-297): the registration request is now persisted to the offline task queue and replayed when the network returns, instead of being dropped fire-once on transient network failures. Customer-visible symptom this fixes: device never gets registered for push ifregisterForRemoteNotificationsis called while the device is offline.Jira: https://iterable.atlassian.net/browse/SDK-108
Changes
RequestProcessorProtocol.swift— addregister(registerTokenInfo:notificationsEnabled:onSuccess:onFailure:)to the shared protocol sosendUsingRequestProcessorcan pick offline vs online at call time. AddRequestIdentifier.registerToken(sibling to the existingdisableDeviceidentifier).RequestHandler.swift— resolvenotificationStateProvider.isNotificationsEnabledfirst, then route throughsendUsingRequestProcessorwith the resolved bool. This snapshotsnotificationsEnabledat call time (parallel to SDK-297'sUserIdentitySnapshot), so a request queued offline and replayed later uses the bool as it was when the caller invoked register — replay does not re-query notification state.OfflineRequestProcessor.swift— implements the newregister(...)using the existingRequestCreator.createRegisterTokenRequest(...)+sendIterableRequest(...)pattern. Drop-in mirror ofdisableDeviceForCurrentUser.OnlineRequestProcessor.swift— publicregister(...)now takesnotificationsEnabled: Booldirectly (provider resolution moved up toRequestHandler); the redundant private helper is folded into the public method; usesRequestIdentifier.registerTokenconsistently.Impact
IterableAPI/InternalIterableAPIregister(token:onSuccess:onFailure:)surface and Obj-C selectors are unchanged.IterableTaskSchedulerCoreData queue instead of dropping the request.notificationsEnabledis frozen at call time. If notification permissions change between schedule and replay, the replayed request still carries the original bool. This matches the SDK-297 precedent and is the desired behavior for queued events.Testing
How to test:
./agent_build.sh(succeeds) and./agent_test.sh(601 unit / 12 notification-extension / 92 offline-events, 0 failures).testOfflineRegisterTokenReplaysWithExpectedBody— confirms an offline-mode register persists and replays with the expected body.testOfflineRegisterTokenRetriesAfterNetworkFailureWithSnapshottedNotificationsEnabled— sends a register, fails the first network attempt, mutatesMockNotificationStateProvider.enabledfromtrue → false, asserts both the failed first send and the replayed second send carrynotificationsEnabled: true(the snapshotted value) and the task is removed from the queue after the successful replay.registerForRemoteNotifications; turn airplane mode off; observe oneregister-device-tokenrequest fired with the original notification-enabled bool.